Skip to content

fix: address 4 quality-debt findings in plugin-loader-helper.sh#3978

Merged
marcusquinn merged 1 commit intomarcusquinn:mainfrom
johnwaldo:bugfix/plugin-loader-fixes
Mar 9, 2026
Merged

fix: address 4 quality-debt findings in plugin-loader-helper.sh#3978
marcusquinn merged 1 commit intomarcusquinn:mainfrom
johnwaldo:bugfix/plugin-loader-fixes

Conversation

@johnwaldo
Copy link
Contributor

@johnwaldo johnwaldo commented Mar 9, 2026 •
edited by coderabbitai bot
Loading

Summary

Addresses 4 of 5 findings from issue #3740 (1 finding — duplicate log functions — was already fixed on main).

Changes

Finding Severity Fix
Semver regex missing hyphens medium Allow - in pre-release character class (1.0.0-beta-1 now valid)
Wrong version file path major Read from $AGENTS_DIR/VERSION instead of nonexistent ~/.aidevops/version
cmd_load runs init hook on every invocation high Remove init hook calls from cmd_load — init belongs in install/enable lifecycle, not session load
2>/dev/null on hook calls medium Remove blanket error suppression; `

Verification

  • ShellCheck passes (zero warnings/errors)
  • Each fix verified against current main code

Closes #3740

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Enhanced version validation to accept more flexible prerelease version formats.
    • Improved plugin load hook execution with enhanced error visibility for better troubleshooting.

  • Chores

    • Refined plugin initialization lifecycle and loading workflow for improved reliability.

@gemini-code-assist
Copy link

gemini-code-assist bot commented Mar 9, 2026

Warning

Gemini encountered an error creating the summary. You can try again by commenting /gemini summary.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 9, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d5c141bf-dca9-4bde-aa04-32ebd7076ecd

📥 Commits

Reviewing files that changed from the base of the PR and between 0e1a6bf and b45a2b5.

📒 Files selected for processing (1)
  • .agents/scripts/plugin-loader-helper.sh

Walkthrough

This PR addresses code review feedback by updating .agents/scripts/plugin-loader-helper.sh to correct plugin lifecycle semantics, fix version source resolution, loosen semver validation to support hyphens in prerelease identifiers, and remove stderr suppression from hook output to improve error visibility.

Changes

Cohort / File(s) Summary
Plugin Lifecycle Fixes
.agents/scripts/plugin-loader-helper.sh		 Removed init hook invocations from cmd_load to prevent execution on every session load (init should only run during install/update/enable per plugin lifecycle semantics); updated comments to reflect correct lifecycle guidance.
Version Source & Validation
.agents/scripts/plugin-loader-helper.sh		 Fixed min_aidevops_version check to read from $AGENTS_DIR/VERSION instead of non-existent $HOME/.aidevops/version; updated semver regex pattern to allow hyphens in prerelease identifiers (-[a-zA-Z0-9.-]+).
Hook Output Handling
.agents/scripts/plugin-loader-helper.sh		 Removed 2>/dev/null stderr suppression from load hook invocations to allow error messages to surface; hooks continue to execute with || true guard to prevent script exit on failure.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • #1138 — The source PR containing the original implementation that these feedback-driven corrections address.

Suggested labels

needs-review-fixes

Poem

🔧 Init hooks stood guard on every load,
Now they rest where they belong—on the install road.
Versions found their true path at last,
And errors speak freely, audit trails cast.
Lifecycle semantics, finally right!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title directly and concisely describes the main change: addressing 4 quality-debt findings in the plugin-loader-helper.sh file.
Linked Issues check ✅ Passed The PR successfully addresses 4 of 5 findings from #3740: semver regex allows hyphens, version file path corrected to $AGENTS_DIR/VERSION, init hooks removed from cmd_load, and stderr suppression removed from hook calls.
Out of Scope Changes check ✅ Passed All changes in the PR are directly scoped to addressing the 4 quality-debt findings from #3740; no extraneous modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@marcusquinn marcusquinn mentioned this pull request Mar 9, 2026
Open
@johnwaldo
Copy link
Contributor Author

johnwaldo commented Mar 9, 2026

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 9, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@marcusquinn
Copy link
Owner

marcusquinn commented Mar 9, 2026

This PR is from an external contributor (@johnwaldo). Auto-merge is disabled for external PRs — a maintainer must review and approve manually.

To approve or decline, comment on this PR:

  • approved — removes the review gate and allows merge (CI permitting)
  • declined: <reason> — closes this PR (include your reason after the colon)

@marcusquinn marcusquinn added external-contributor PR from external contributor, requires maintainer review needs-maintainer-review labels Mar 9, 2026
@marcusquinn
Copy link
Owner

marcusquinn commented Mar 9, 2026

Base branch was modified by PR #3975 merge. Dispatching worker to rebase and re-merge.

@marcusquinn
fix: address 4 quality-debt findings in plugin-loader-helper.sh
b45a2b5 
- Fix semver regex to allow hyphens in pre-release identifiers (e.g. 1.0.0-beta-1)
- Fix version file path: read from $AGENTS_DIR/VERSION instead of nonexistent ~/.aidevops/version
- Remove init hook calls from cmd_load (init belongs in install/enable, not session load)
- Remove 2>/dev/null from hook calls to surface errors

Closes marcusquinn#3740
@marcusquinn marcusquinn force-pushed the bugfix/plugin-loader-fixes branch from aacd246 to b45a2b5 Compare March 9, 2026 19:03
@github-actions github-actions bot added the bug Auto-created from TODO.md tag label Mar 9, 2026
@marcusquinn marcusquinn merged commit 40cf572 into marcusquinn:main Mar 9, 2026
19 checks passed
@github-actions github-actions bot mentioned this pull request Mar 9, 2026
Closed
@coderabbitai coderabbitai bot mentioned this pull request Mar 12, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] approved these changes

Assignees

No one assigned

Labels

bug Auto-created from TODO.md tag external-contributor PR from external contributor, requires maintainer review needs-maintainer-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

quality-debt: .agents/scripts/plugin-loader-helper.sh — PR #1138 review feedback (high)

2 participants

@johnwaldo @marcusquinn